Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support non base type items in structs #343 #465

Closed
wants to merge 17 commits into from

Conversation

mjobuda
Copy link
Contributor

@mjobuda mjobuda commented Jun 22, 2021

What was wrong?

Support non base type items in structs #343

How was it fixed?

  • added support for string type items in structs
  • developed a unit test for this

To-Do

@codecov-commenter
Copy link

codecov-commenter commented Jun 22, 2021

Codecov Report

Merging #465 (21f5993) into master (b0caf12) will decrease coverage by 0.79%.
The diff coverage is 34.24%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #465      +/-   ##
==========================================
- Coverage   87.17%   86.38%   -0.80%     
==========================================
  Files          73       73              
  Lines        5013     5067      +54     
==========================================
+ Hits         4370     4377       +7     
- Misses        643      690      +47     
Impacted Files Coverage Δ
analyzer/src/traversal/structs.rs 42.85% <31.42%> (-54.02%) ⬇️
compiler/src/yul/runtime/functions/structs.rs 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0caf12...21f5993. Read the comment docs.

@sbillig
Copy link
Collaborator

sbillig commented Jun 22, 2021

@mjobuda Small request: could you rebase your relevant commits onto a branch that tracks ethereum/fe/master? It would be nicer to review these PRs if there weren't unrelated commits included. It's not a big deal, because the gh "Files changed" view is clean, but for more complicated changes, I often pull down PRs and step through the individual commits in my editor.

To be clear, this is a very minor thing and I'm still happy to review contributions if the commit history isn't clean.

@mjobuda
Copy link
Contributor Author

mjobuda commented Jun 23, 2021

@sbillig I'd love to!
Can you give me some instructions on how to
"rebase my relevant commits onto a branch that tracks ethereum/fe/master?".
Is there a magic git command for this?

@cburgdorf
Copy link
Collaborator

Can you give me some instructions on how to

Basically:

git fetch
git rebase origin/master

You might have to work through conflicts during rebase in which case you:

  1. Resolve them
  2. Stage the files (e.g. git add .)
  3. Run git rebase --continue
  4. Repeat from 1.) if needed until the rebase is done

Then you you push your branch again with the -f parameter (force push)

@mjobuda
Copy link
Contributor Author

mjobuda commented Jun 24, 2021

@cburgdorf i'm still not sure if i got it right, sorry but i don't get stuff like
https://stackoverflow.com/questions/9257533/what-is-the-difference-between-origin-and-upstream-on-github
What is the correct way to get the work done and not get into this kind of problems?
Should I clone from the master and then create my branch?
Or git branch --set-upstream-to=origin/master i193 ???
What are the proper steps to not run into this kind of problems.
I see still all my commits and have no idea what and how to do it to not get git come into my way.
Sorry, but I don't get git.
I have no idea how to get a clean working environment.
Whatever I do, I still have "This branch is 10 commits ahead of ethereum:master.".
Is there a simple way to get rid of this?
Should I delete my fork?
If so how should I proceed to not have this git problems

@cburgdorf
Copy link
Collaborator

Ah, sorry, maybe I confused what origin is in your case. In fact, the term origin is heavily overloaded which is why I personally never use it. I use me to point to my own fork and upstream to point to the central repository that everyone uses to coordinate the work.

Let's try that again.

  1. Run git remote -v

Does this only show origin and is that your own fork?

  1. Run git remote add upstream https://github.com/ethereum/fe.git to add an upstream pointing to this repo.

  2. Fetch the latest from upstream via git fetch upstream

  3. Rebase git rebase upstream/master

You might have to work through conflicts during rebase in which case you:

I. Resolve them
II. Stage the files (e.g. git add .)
III. Run git rebase --continue
IV. Repeat from I.) if needed until the rebase is done

  1. Then force push your branch into your fork via git push -f origin <branchname>

If that still doesn't work I'm happy to hop on a call with you or just feel free to join our team call today at 17:00 Berlin time :)

@sbillig
Copy link
Collaborator

sbillig commented Dec 17, 2021

I'm going to close this, as it seems to be abandoned (perhaps because I derailed it with git nonsense 😬). @mjobuda, please don't hesitate to reopen this if you're still interested.

@sbillig sbillig closed this Dec 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants